Skip to content

Conversation

hkratz
Copy link
Contributor

@hkratz hkratz commented Jan 30, 2022

std::fs::remove_dir_all() is recursive and can run out of stack space for deep directory hierarchies. This uses @cuviper's looped implementation with an on-heap stack to avoid running out of stack space before running out of file descriptors. As a drive-by fix it reduces the Macos x86-64 special case code.

For easier review the Macos x86-64 commits are broken up in two. One removes the Macos x86-64 special case completely, the second adds more fine-grained special-case handling back.

Further work to make remove_dir_all() use only a fixed number of open file descriptors is planned, see #93160.

@rust-highfive
Copy link
Contributor

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 30, 2022
@hkratz hkratz changed the title Convert std::fs::remove_dir_all() from recursive to looping Convert UNIX std::fs::remove_dir_all() from recursive to looping Jan 30, 2022
@cuviper
Copy link
Member

cuviper commented Jan 31, 2022

+1 for consolidating macos -- always nice to see a net reduction in code!

@hkratz
Copy link
Contributor Author

hkratz commented Feb 28, 2022

Needs to be rebased once #94446 is merged.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 28, 2022
@bors
Copy link
Collaborator

bors commented Mar 5, 2022

☔ The latest upstream changes (presumably #94634) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnCSimon
Copy link
Member

@hkratz

Needs to be rebased once #94446 is merged.

Triage: Looks like this is unblocked - can you post your status on this PR?

@hkratz
Copy link
Contributor Author

hkratz commented Apr 11, 2022

Closing in favor of #95925.

@hkratz hkratz closed this Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants